@alikon @muhme Could you test this one here? Just check if the changes from #310 are still working and if IP v6 addresses enclosed into square brackets are working. That would be much appreciated.

@alikon @Hackwar @HLeithner @muhme and possible other other reviewers: Please check the "Request for comments - RFC" section in the description and let me know your opinion. Thanks in advance.

Running System Tests with current 5.2-dev and JBT version 1.1.15 and bash command line:

1. Create 5.2-dev IPv6 installation with all needed patches

# joomla-cms-43968 and joomla-cms-44084 are already merged in 5.2
scripts/create 52 IPv6 database-310 database-316 joomla-cypress-33 joomla-cypress-36

2. Preparing tests with non-default database ports by setting up three port forwardings in the Cypress container:

docker exec jbt_cypress bash -c 'apt-get update && apt-get upgrade -y && apt-get install socat -y'
docker exec -d jbt_cypress socat 'TCP6-LISTEN:4711,fork,reuseaddr' 'TCP6:[fd00::11]:3306'
docker exec -d jbt_cypress socat 'TCP6-LISTEN:4712,fork,reuseaddr' 'TCP6:[fd00::12]:3306'
docker exec -d jbt_cypress socat 'TCP6-LISTEN:4713,fork,reuseaddr' 'TCP6:[fd00::13]:5432'

3. ✅ Test IPv6 with non-default port number using test.sh as one-liner script with Cypress configuration, the Web Installer installation step, and one test specification that uses custom database commands.

4. ✅ Test IPv6 with unset port number by adapting test.sh and running it again:

types=("MySQLi" "MySQL (PDO)" "MySQLi" "MySQL (PDO)" "PostgreSQL (PDO)")
hosts=("fd00::11" "fd00::11" "fd00::12" "fd00::12" "fd00::13")
ports=("" "" "" "" "")

5. ✅ Test IPv4 with unset port number by adapting test.sh and running it again:

types=("MySQLi" "MySQL (PDO)" "MySQLi" "MySQL (PDO)" "PostgreSQL (PDO)")
hosts=("10.0.0.11" "10.0.0.11" "10.0.0.12" "10.0.0.12" "10.0.0.13")
ports=("" "" "" "" "")

6. ✅ Test IPv4 with non-default port number by adapting test.sh and running it again:

types=("MySQLi" "MySQL (PDO)" "MySQLi" "MySQL (PDO)" "PostgreSQL (PDO)")
hosts=("10.0.0.7" "10.0.0.7" "10.0.0.7" "10.0.0.7" "10.0.0.7")
ports=("4711" "4711" "4712" "4712" "4713")

7. ✅ Test hostname with unset port number by adapting test.sh and running it again:

types=("MySQLi" "MySQL (PDO)" "MySQLi" "MySQL (PDO)" "PostgreSQL (PDO)")
hosts=("jbt_mysql" "jbt_mysql" "jbt_madb" "jbt_madb" "jbt_pg")
ports=("" "" "" "" "")

8. ✅ Test hostname with non-default port number by adapting test.sh and running it again:

types=("MySQLi" "MySQL (PDO)" "MySQLi" "MySQL (PDO)" "PostgreSQL (PDO)")
# Using docker container name and not 'jbt_cypress' as it’s an invalid hostname (will be fixed in the next JBT version).
hosts=("77b3b08bd2ff" "77b3b08bd2ff" "77b3b08bd2ff" "77b3b08bd2ff" "77b3b08bd2ff")
ports=("4711" "4711" "4712" "4712" "4713")

9. 🟥 Test Unix sockets by adapting test.sh and running it again:

types=("MySQLi" "MySQL (PDO)" "MySQLi" "MySQL (PDO)" "PostgreSQL (PDO)")
hosts=("unix:/var/run/mysql-socket/mysqld.sock" "unix:/var/run/mysql-socket/mysqld.sock" "unix:/var/run/mariadb-socket/mysqld.sock" "unix:/var/run/mariadb-socket/mysqld.sock" "unix:/var/run/postgresql-socket")
ports=("" "" "" "" "")

Failed for mysqli and mariadbi with 500 server error with:

Joomla\Database\DatabaseDriver::extractHostPortSocket(): Argument #1 ($host) must be of type string, null given, called in /var/www/html/libraries/vendor/joomla/database/src/Mysqli/MysqliDriver.php on line 203

Hmm the MySQLi and the PDO driver set the host to „localhost“ in the constructor, so a null host value should never happen.

Will check later why that fails.

To verify, I created a new installation using the database-315 patch instead, ran test No.9 Unix sockets and it is successfully passed.

Yes, I know what the problem is. The method can run multiple times when connecting for checking the database and then disconnecting and connecting again to make the installation.

In the old code before PR #310 the $this->options['host'] set to null at the first run and then was just passed again to the preg_replace at the 2nd run, which causes a deprecation notice on newer PHP version but not an error.

Now we have a function where the parameter is not nullable.

There are 2 ways to solve it: Either I do it like for PDO and do not modify the $this->options but use local variables, so we have the same starting conditions with every new connect. Or I make the $hostparameter nullable and do an early return of the unmodified parameters when that parameter is null.

I tend to the first way.

The real clean way would be to use the method not in the connect method but in the constructor so it would run only one time on a particular instance.

Hmm, on the other hand, the 2nd way would make it behave on MySQL like it was before PR #310 .

9. 🟥 Test Unix sockets by adapting `test.sh` and running it again:

types=("MySQLi" "MySQL (PDO)" "MySQLi" "MySQL (PDO)" "PostgreSQL (PDO)")
hosts=("unix:/var/run/mysql-socket/mysqld.sock" "unix:/var/run/mysql-socket/mysqld.sock" "unix:/var/run/mariadb-socket/mysqld.sock" "unix:/var/run/mariadb-socket/mysqld.sock" "unix:/var/run/postgresql-socket")
ports=("" "" "" "" "")

Failed for mysqli and mariadbi with 500 server error with:

Joomla\Database\DatabaseDriver::extractHostPortSocket(): Argument #1 ($host) must be of type string, null given, called in /var/www/html/libraries/vendor/joomla/database/src/Mysqli/MysqliDriver.php on line 203

@muhme Fixed. Thanks for testing so far.

✅ Successfully completed the test with all 9 steps again, confirming that all tests passed smoothly this time. 👍

I have some questions.

1. It seems hard to validated a ipv4 or ipv6 address with all it's implications, maybe using a lib for it is a better approach or at least look at it and make own code better, my first hit was https://github.com/mlocati/ip-lib no idea if it's good but it also support ipv6 zones
2. What's the reason todo the detection on connect and not in the constructor? If I saw it right we have no function to change the options later, and if we have we might simple run an HostPortWhateverupdate function with the new DSN string?

wouldn't this reduce a bit of complexity? at least I read somewhere something about double decode issue.

@HLeithner Regarding your question 2: I don’t know why it is done in the connect method. It was like that since long time, if not since ever, when I checked the history on the 1.0-dev branch. I did not want to change that in a patch version. I fully agree that it should be done in the constructor. If there is consensus about that I can change this PR here (or make a new one).

Regarding question 1: Same reason as question 2, I did not want to make such a big change now.

We could discuss that in the next maintainers meeting for framework topics.

I've created an alternative PR which makes the same changes as this one here (except of adding a unit test) plus moves the modification of options from the connect method to the constructor.

Closing in favour of #317 . Thanks for feedback and tests.